Skip to content

Distrust received pack indexes (behind config flag, with perf fixes) #1846

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 24, 2025

Conversation

tyrielv
Copy link
Contributor

@tyrielv tyrielv commented Jun 23, 2025

This pull request contains 4 things:

  1. Always index locally for prefetch packs #1840 to index packs locally, which was reverted due to unexpected performance issues
  2. Index prefetch packs in parallel #1843 to increase parallelization when indexing packs, which was abandoned due to Always index locally for prefetch packs #1840 being reverted
  3. Added a new configuration "gvfs.trust-pack-indexes", which defaults to true. The new behavior is only used when this is set to false.
    This is intended to be a temporary configuration setting, to be removed (or defaulted to false) once more mitigations have been completed for the performance.
  4. git index-pack is changed from running in the GVFS enlistment to running outside the enlistment. This was the reason for the unexpected performance issues.
    It was expected that the first prefetch on a new clone would take longer due to indexing the pack locally; however users who deleted their prefetch cache (but not the rest of the cache or local loose objects) in order to re-fetch it with local indexing enabled experienced many times longer delays than expected, because git index-pack reads all the existing pack indexes and loose objects and considers them when indexing a pack in order to support validating that all referenced objects exist - even when the command-line options to act on nonexistent references are not enabled. Since we aren't using --validate or its variants, we can run git index-pack outside the enlistment to avoid this issue.

tyrielv added 3 commits June 19, 2025 09:08
The GVFS protocol includes an index file along with pack file in the prefetch
workflow (primarily used on a new clone to fetch all commits and trees).

Currently, GVFS blindly accepts that index file.

This pull request changes GVFS prefetch to discard the index sent by the
server and always create an index locally. This provides improved security
and verification of the pack file, at the expense of performance for the first
clone of a repository on a new drive.
Copy link
Member

@mjcheetham mjcheetham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one question (and one nit pick 😄) about thread count.

@mjcheetham mjcheetham requested a review from dscho June 23, 2025 15:54
@mjcheetham mjcheetham merged commit 26b88aa into microsoft:master Jun 24, 2025
5 checks passed
@mjcheetham mjcheetham mentioned this pull request Jun 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants